-
Notifications
You must be signed in to change notification settings - Fork 81
DOC: minor tweak to docs on invoking pypa/build #707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
We have several other example |
|
I think we should add |
717af2c to
0e1ee55
Compare
0e1ee55 to
cf6e2a7
Compare
|
@dnicolodi this is ready, CI failures were unrelated - PTAL, would be nice to merge it. |
| If the build succeeded, you'll have the binary artifacts in the ``dist`` folder. | ||
| Note that by default, ``python -m build`` builds an sdist first, and then a | ||
| wheel from the sdist. If you only want one artifact, add ``--sdist`` or | ||
| ``--wheel`` to the invocation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this misses the main point. The issue in not the extra work for building an sdist that will not be used, but that doing so any uncommitted change in the repository is not reflected in the wheel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good point, will add that as the first point since it's indeed a footgun. I think both can be relevant; sdist creation for a large project can be really slow (a quick unscientific measurement says 40 seconds for scipy at the moment, and it might breaking caching on rebuilds as well).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a note on uncommitted changes two paragraphs up, where it already talks about Git.
cf6e2a7 to
ad68bf0
Compare
| to your Git repository - ``meson-python`` will only take into account the files | ||
| that Git knows about. | ||
| that Git knows about, and an sdist is created from the most recent commit - | ||
| uncommitted changes are ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is very confusing. What is committed only affects what gets into an sdist, but this section is actually about building a wheel. I think this paragraph needs to be removed and the note about uncommitted changes moved below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's about both sdist and wheels. It's part of the "Creating your first release" section, and clearly does talk about both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to change it to an admonition while we're at it, but I really just want to get this PR in - it was about adding the --wheel flag and not related to the commit status of files at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not make this paragraph worse, so we can merge this PR and fix this part of the documentation later.
Addresses a comment on issue 704, where a user got tripped up by `python -m build` building an sdist first (and that's typically not what you want).
ad68bf0 to
7ee01a1
Compare
|
I took the liberty of fixing the commit title (this does not touch how pip is invoked, thus I think the commit title was a typo) |
Addresses a comment on gh-704, where a user got tripped up by
python -m buildbuilding an sdist first (and that's typically not what you want).